Skip to content

chore!: remove orphan Meteor (DDP) methods scheduled for 9.0.0#40656

Draft
ggazzo wants to merge 9 commits into
release-9.0.0from
chore/ddp-orphan-methods-removal
Draft

chore!: remove orphan Meteor (DDP) methods scheduled for 9.0.0#40656
ggazzo wants to merge 9 commits into
release-9.0.0from
chore/ddp-orphan-methods-removal

Conversation

@ggazzo

@ggazzo ggazzo commented May 22, 2026

Copy link
Copy Markdown
Member

Summary

Removes 94 Meteor (DDP) method registrations that have no callers anywhere in this repository. These were already flagged for removal in 9.0.0 on develop via methodDeprecationLogger.method(name, '9.0.0', ...) (#40654).

Method files that ended up containing only imports + a declare module '@rocket.chat/ddp-client' block after the removal are deleted entirely, and matching side-effect imports are stripped from sibling index.ts barrels.

Counters

  • 94 method keys removed across 90 files
  • 23 method files deleted entirely
  • 22 sibling barrel imports cleaned up
  • Resulting diff: 100 files changed, 9 insertions, 2264 deletions
  • ESLint on changed files: 0 errors, only pre-existing warnings

How

Three scripts under scripts/:

  • audit-ddp-methods.mjs — enumerates Meteor methods + callers + REST routes, produces docs/ddp-audit.{json,md}.
  • remove-orphan-ddp-methods.mjs — strips orphan keys from Meteor.methods({...}) blocks; if the block becomes empty, drops the whole statement.
  • cleanup-after-orphan-removal.mjs — prunes unused imports, deletes files that are now nothing-but-imports + declare module, removes barrel side-effect imports.

Skip list (intentionally kept)

The following methods are reachable indirectly via the admin settings UI (apps/meteor/client/views/admin/settings/Setting/inputs/MethodActionInput.tsx:12 calls useMethod(value) where value is the setting's stored method name) or via cloud admin actions. They look orphan to the string-grep audit but are still callable, so they are excluded:

OEmbedCacheCleanup, restart_server, push_test, crowd_sync_users, crowd_test_connection, loadLocale, resetIrcConnection, checkFederationConfiguration, removeSlackBridgeChannelLinks, cloud:checkRegisterStatus, cloud:registerWorkspace, cloud:checkUserLoggedIn, cloud:logout, cloud:finishOAuthAuthorization, cloud:getOAuthAuthorizationUrl.

Risk

This is the destructive half of the DDP-removal effort started on develop. The audit only matches literal Meteor.call('name', ...) strings — anything calling these methods with a dynamic name, from the mobile SDK, or from a third-party client that talks DDP directly, will start failing as method-not-found. Reviewers should flag any method below that they know is invoked externally.

declare module '@rocket.chat/ddp-client' { interface ServerMethods { ... } } augmentations for the removed methods are still present in some surviving files (the type entries are not load-bearing now that no implementation exists). They can be swept up in a follow-up if desired.

Test plan

  • Confirm apps/meteor builds and typechecks
  • Spot-check that none of the 94 removed methods are called from a partner mobile/SDK consumer
  • Verify admin settings buttons (push test, restart server, crowd sync, IRC reconnect, federation check, OEmbed cache cleanup, etc.) still work — these are on the skip list and should be unaffected
  • Verify the cloud register/connect/logout/OAuth admin flows still call DDP correctly (skip-listed)

@dionisio-bot

dionisio-bot Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot

changeset-bot Bot commented May 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 75cc268

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@rocket.chat/meteor Major
@rocket.chat/core-typings Major
@rocket.chat/rest-typings Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f2dab235-8fb5-4fec-8b06-102b3c45956b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.11%. Comparing base (1a6aafa) to head (75cc268).

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##           release-9.0.0   #40656      +/-   ##
=================================================
+ Coverage          70.05%   70.11%   +0.06%     
=================================================
  Files               3355     3348       -7     
  Lines             129153   128918     -235     
  Branches           22385    22323      -62     
=================================================
- Hits               90475    90388      -87     
+ Misses             35384    35242     -142     
+ Partials            3294     3288       -6     
Flag Coverage Δ
e2e 59.19% <ø> (-0.16%) ⬇️
e2e-api 46.74% <ø> (+0.48%) ⬆️
unit 69.96% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ggazzo added a commit that referenced this pull request May 22, 2026
The deprecation logger throws in TEST_MODE
(apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts:118), so a
deprecation log on a method that's still invoked by integration or
unit tests turns those tests into failures with messages like:

    Error: The method "addUsersToRoom" is deprecated and will be
    removed on version 9.0.0

That's the intended dev-loop guard rail, but it means this PR can
only carry deprecation calls on methods with **no callers**.

Removes the 83 logger calls (and their now-unused imports) that
targeted methods listed in audit.used. They will be revisited as part
of the caller-migration PR #40659 — once each callsite moves to the
matching REST endpoint, the DDP method can either get a deprecation
log here or be removed outright in #40656.

What remains in this PR: 71 deprecation calls, all on orphan methods
without a REST replacement.
ggazzo added a commit that referenced this pull request May 25, 2026
… proxy

These 5 methods were classified as orphan by the static-string caller
audit but are actually invoked at runtime from API tests through the
`/v1/method.call/:method` REST proxy. Logging a deprecation on them
makes the TEST_MODE throw fire in CI even with TEST_MODE_API set,
because the proxy path runs before the env-gated suppression hits
the test step.

Removes the logger calls and now-unused imports for:
  - cleanRoomHistory
  - getUsersOfRoom
  - readMessages
  - setUserActiveStatus
  - updateMessage

These methods are removed entirely in #40656 (release-9.0.0); deprecation
logging on develop is therefore redundant. The audit's missing-caller
heuristic should be extended in a follow-up to recognize callers that
only exist in the test suite.
@rc-layne

rc-layne Bot commented May 25, 2026

Copy link
Copy Markdown

Warning

These are security findings reported by the security scanners configured in Layne. Findings may contain false positives - review them and fix what makes sense.

Layne found 1 high issue in this PR.

View 1 finding(s)
Severity Scanner File Rule Description
🟠 High semgrep scripts/cleanup-after-orphan-removal.mjs:29 app.config.semgrep.rules.nodejs.child-process-execution Child process execution detected. Ensure command and arguments are not user-controlled. Command injection (CWE-78) can occur if untrusted input flows into spawn/exec calls. Remediation: - Use allowlists for permitted commands - Validate/sanitize all arguments - Avoid shell=True or string-based command interpolation - Prefer execFile/spawn over exec when possible

@julio-rocketchat

Copy link
Copy Markdown
Member

/layne exception-approve LAYNE-6ceb2179c9a0541a reason: hardcoded command - it's not generated dynamically and poses no risk as is

@rc-layne

rc-layne Bot commented May 25, 2026

Copy link
Copy Markdown

✅ Exception recorded for LAYNE-6ceb2179c9a0541a by @julio-rocketchat: "hardcoded command - it's not generated dynamically and poses no risk as is". Re-running scan...

ggazzo added a commit that referenced this pull request May 25, 2026
… proxy

These 5 methods were classified as orphan by the static-string caller
audit but are actually invoked at runtime from API tests through the
`/v1/method.call/:method` REST proxy. Logging a deprecation on them
makes the TEST_MODE throw fire in CI even with TEST_MODE_API set,
because the proxy path runs before the env-gated suppression hits
the test step.

Removes the logger calls and now-unused imports for:
  - cleanRoomHistory
  - getUsersOfRoom
  - readMessages
  - setUserActiveStatus
  - updateMessage

These methods are removed entirely in #40656 (release-9.0.0); deprecation
logging on develop is therefore redundant. The audit's missing-caller
heuristic should be extended in a follow-up to recognize callers that
only exist in the test suite.
ggazzo added a commit that referenced this pull request May 25, 2026
… proxy

These 5 methods were classified as orphan by the static-string caller
audit but are actually invoked at runtime from API tests through the
`/v1/method.call/:method` REST proxy. Logging a deprecation on them
makes the TEST_MODE throw fire in CI even with TEST_MODE_API set,
because the proxy path runs before the env-gated suppression hits
the test step.

Removes the logger calls and now-unused imports for:
  - cleanRoomHistory
  - getUsersOfRoom
  - readMessages
  - setUserActiveStatus
  - updateMessage

These methods are removed entirely in #40656 (release-9.0.0); deprecation
logging on develop is therefore redundant. The audit's missing-caller
heuristic should be extended in a follow-up to recognize callers that
only exist in the test suite.
ggazzo added a commit that referenced this pull request May 26, 2026
The deprecation logger throws in TEST_MODE
(apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts:118), so a
deprecation log on a method that's still invoked by integration or
unit tests turns those tests into failures with messages like:

    Error: The method "addUsersToRoom" is deprecated and will be
    removed on version 9.0.0

That's the intended dev-loop guard rail, but it means this PR can
only carry deprecation calls on methods with **no callers**.

Removes the 83 logger calls (and their now-unused imports) that
targeted methods listed in audit.used. They will be revisited as part
of the caller-migration PR #40659 — once each callsite moves to the
matching REST endpoint, the DDP method can either get a deprecation
log here or be removed outright in #40656.

What remains in this PR: 71 deprecation calls, all on orphan methods
without a REST replacement.
ggazzo added 8 commits June 11, 2026 15:43
- scripts/audit-ddp-methods.mjs: enumerates Meteor.methods registrations,
  callers, and REST replacements, producing docs/ddp-audit.{json,md}.
- scripts/remove-orphan-ddp-methods.mjs: removes orphan method keys from
  Meteor.methods({...}) blocks; if the block becomes empty, drops the
  whole statement. Uses a skip-list for methods reachable via admin
  MethodActionInput dynamic dispatch.
- scripts/cleanup-after-orphan-removal.mjs: prunes the now-unused imports
  in files we just edited, deletes files that no longer contain any code
  besides imports and declare-module blocks, and removes the matching
  side-effect imports from barrel index.ts files.
Removes 94 Meteor method registrations that had no callers anywhere in
the repository — they were marked for removal in 9.0.0 by the
deprecation logging shipped earlier on develop. Files that ended up
containing only imports and declare-module blocks after the removal
were deleted, and matching side-effect imports were stripped from
sibling index.ts barrels.

The removal keeps a skip-list of methods that are still reachable
indirectly: admin settings buttons that dispatch via useMethod(value)
in MethodActionInput.tsx, and a handful of cloud:* methods invoked from
admin actions. Those will be revisited once their callers move to REST.

Removed by scripts/remove-orphan-ddp-methods.mjs +
scripts/cleanup-after-orphan-removal.mjs from docs/ddp-audit.json.

BREAKING CHANGE: 94 DDP methods previously registered are no longer
callable. Any DDP-only client that still relies on Meteor.call(<name>)
for these methods must migrate to the corresponding REST endpoint
(see docs/ddp-audit.md for the mapping) or, where no REST equivalent
exists, drop usage.
The first pass of the orphan removal triggered three classes of CI
failures:

1. `@typescript-eslint/no-unused-vars` errors where the import-pruner
   removed type imports that were still referenced inside the
   `declare module '@rocket.chat/ddp-client' { interface ServerMethods
   { ... } }` augmentation blocks. The pruner was stripping declare
   module content before checking name usage, so types like
   `IImport`, `ICreatedRoom`, `IMessageSearchProvider`,
   `RoomType`, `RocketChatRecordDeleted`, and `WithId` were
   incorrectly marked unused.

2. `@typescript-eslint/no-unused-vars` errors where the interface
   ServerMethods entries for the removed methods were still pointing
   at types we had already imported. The new
   scripts/strip-orphan-interface-entries.mjs walks each declare
   module block in step with the removal plan and drops the matching
   interface entries (and the whole declare module block if it
   becomes empty), keeping the implementation removal and the type
   surface in sync. A skip-list inside the script restores entries
   for the 8 methods that turned out to have callers we missed.

3. `error TS2345: Argument of type '...' is not assignable to
   parameter of type 'keyof ServerMethods'` for methods that the
   audit reported as orphan but were called through dynamic dispatch
   the literal-string scanner couldn't see:

     - `permissions/get`, `rooms/get`, `subscriptions/get`,
       `public-settings/get`, `private-settings/get` are called
       via a template literal in
       apps/meteor/client/lib/cachedStores/CachedStore.ts as
       `sdk.call(\`${this.name}/get\`)`.
     - `samlLogout` is called via `sdk\n.call('samlLogout', ...)`
       in apps/meteor/client/meteor/login/saml.ts — the audit's
       `\bsdk\.` regex didn't tolerate whitespace between
       `sdk` and `.call`.
     - `blockUser` and `unblockUser` are called via a conditional
       `useMethod(isUserBlocked ? 'unblockUser' : 'blockUser')` in
       apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useBlockUserAction.ts,
       which the scanner can't reach.

   Restoring those 8 implementations and adjusting the matching barrel
   imports puts the runtime contract back together. The scripts'
   detection patterns will be hardened in a follow-up.

Also drops apps/meteor/app/push/server/methods.ts and its
`import './methods'` from `apps/meteor/app/push/server/index.ts`:
the file's last DDP entry was removed in an earlier pass, leaving the
file with nothing but an unused `PushUpdateOptions` alias and an
orphan interface declaration.
apps/meteor/server/importPackages.ts:71 imports
../app/meteor-accounts-saml/server, so the directory needs an index.ts
that loads the side-effect modules. The previous cleanup pass deleted
the barrel when it looked dead, breaking lint and the runtime require.
Lists the 87 Meteor (DDP) methods this PR removes, with the
matching REST endpoint where one exists. Major bump on
@rocket.chat/meteor flags the breaking-change semantics for
external DDP/SDK clients.
The tagged describe blocks in tests/end-to-end/api/methods.ts that
exercised method.call/:method directly against the removed methods
no longer have anything to test - the implementations are gone in
this PR. Removed the four blocks (1005 lines):

  - [@cleanRoomHistory]
  - [@getUsersOfRoom]
  - [@updateMessage]
  - [@setUserActiveStatus]

Two unrelated test files used `saveUserPreferences` as a setup step
before exercising a different feature. Migrated those callsites to
POST /v1/users.setPreferences so the test still does what it intended
once the DDP method is gone:

  - tests/end-to-end/api/direct-message.ts: setting emailNotificationMode
    before creating a DM.
  - tests/end-to-end/api/teams.ts: setting emailNotificationMode before
    inviting the user to a team.

The `methodCall` import in direct-message.ts was the only remaining
reference; dropped it to satisfy the unused-vars rule.
UI tests cover the forgot-password flow which triggers
`sendForgotPasswordEmail` through Meteor's `Accounts.forgotPassword`
client API — the call chain is invisible to the audit's static
string-literal scan.

Restores the method file from origin/release-9.0.0, drops the entry
from the changeset (count 87 -> 86), and adds it to the NEVER_REMOVE
skip list in scripts/remove-orphan-ddp-methods.mjs alongside the
other dynamic-dispatch survivors so a future audit run doesn't try
to remove it again.
The [@getReadReceipts] EE test used readMessages as a fixture step to
simulate the invited user reading the main room before checking that
both sender and recipient receipts show up. readMessages is removed
in this PR; the test now POSTs /v1/subscriptions.read instead, which
is the supported REST equivalent.
@ggazzo ggazzo force-pushed the chore/ddp-orphan-methods-removal branch from 8407e3c to 8651158 Compare June 11, 2026 19:05
The deprecation warning calls were removed alongside the orphan DDP
methods, leaving dangling imports that broke TypeScript (TS6133) and
ESLint (no-unused-vars).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants